-
-
Notifications
You must be signed in to change notification settings - Fork 341
fix: cargo-auditable build error #1929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
I think there needs to be a comment around these, otherwise these will seem like an oversight and changed back sometime in the future. These comments should contain links for more context.
#1024 is a decent description of what's going on, and it doesn't look like the reported bug in Cargo is fixed yet.
Use `futures-lite` instead of `dep:futures-lite` in gix-packetline and gix-protocol. See-Also: rust-secure-code/cargo-auditable#124
25b6e3d
to
11bd59f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! I've verified that, starting after this was merged, cargo auditable build -r
works to build gitoxide
on GNU/Linux, macOS, and Windows.
Now that cargo auditable
is working, I hope we will eventually be able to use it for the release binaries built and published on GitHub (see release.yml
). However, I think this might not be immediately feasible, because the use of cross
for all the Linux builds currently gets in the way (rust-secure-code/cargo-auditable#95 (comment)).
One thing that seems strange to me about that error is that it's looking for a |
Hi @Byron, I initially had an idea of creating a empty feature instead of dropping dep:, could you try if that works? |
Yes, it's very strange. I would have trouble thinking that
Thanks for trying to help! I couldn't really apply this in a straightforward fashion, I don't understand what it is doing, but experimented more. From there I can say that |
You can get the specific invocation. The left sub-panel shows this tree view:
As shown, you have Failed to select a version for 'git-packetline-blocking' selected. If you click on Build script evaluation, then the output on the right should show a command at the top. Currently I only have RustRover on Windows, but I am able to produce this there. The full contents of the right sub-panel, when Build script evaluation is selected on the left sub-panel, is the following, in which the first (very long) line is the
Whatever is going on, it should be possible to figure it out from the command itself, because running
Edit: What I don't know how to do through RustRover itself is to find out what the Edit 2: It turns out that showing the command when there is no error is straightforward too: right-clicking on the top item in the tree view in the left sub-panel shows a contextual menu one of whose items is "Show Successful Steps". Click that populates the tree and then the items can be expanded, including the "Build script evaluation" item. This reveals that, without the patch here, the command RustRover runs is:
Using this diff tool reveals that the difference is that, when the changes from this PR are present, RustRover includes Of those, the first, cargo check --workspace --features gix-packetline/futures-lite The output is:
If this command is something that should work, then further changes will be needed, and if they cannot be made, then this PR could be reverted--though my guess is that knowing Edit 3: I believe the problem arises because gitoxide/gix-filter/Cargo.toml Line 25 in 705b86d
Based on this, I think there are three potentially reasonable ways forward, any of which should fortunately both preserve the changes in this PR and make RustRover usable again:
The current situation seems sufficiently similar to #1123, especially with respect to the motive for adding |
This adds a `futures-lite` feature to `gix-packetline-blocking`. The new feature is undocumented except for the warning not to use it. It does nothing, and its purpose is to support an existing internal use of `gix-packetline-blocking` through an alias named `gix-packetline`. This new feature should never be used except to keep `cargo check` commands with `--workspace` that list `gix-packetline/futures-lite` from failing due to the absent feature. Such `cargo check` commands are rare and `cargo check` should not typically be used this way. But RustRover automatically composes and runs such a command. This fixes a RustRover project discovery breakage commented on in GitoxideLabs#1929. This "ghost feature" may be removed without warning. Nothing should rely on it in production or otherwise in a significant way. It is a bug for any software to break or change behavior if it is removed. This addition is similar to the addition in be4de0d (GitoxideLabs#1123) of the `async-io` feature of `gix-packetline-blocking`, which likewise shouldn't be used. However, `gix-packetline-blocking/futures-lite` is even less elegant than `gix-packetline-blocking/async-io`, since `gix-packetline` doesn't explicitly delcare a `futures-lite` feature. Instead, we currrently don't use `dep:` for `futures-lite` because it breaks `cargo-auditable` (GitoxideLabs#1929).
This removes the `gix-packetline-blocking/futures-lite` "ghost feature" added in d5dd239 (GitoxideLabs#1939), and instead: - Changes `gix-filter` to depends on `gix-packetline-blocking` without aliasing it `gix-packetline`. - Replaces uses of `gix_packetline` with `gix_packetline_blocking` in the code of `gix-filter`. Thus, this replaces the solution in GitoxideLabs#1939 for the problem discussed in GitoxideLabs#1929 (comment) with a different solution that avoids carrying a "ghost feature." In contrast, the long-standing `gix-packetline-blocking/async-io` feature added in be4de0d (GitoxideLabs#1123), though it should not be used, is *not* removed, because: - It is referenced in attributes in `gix-packetline-blocking` code (because `gix-packetline-blocking/src` is copied from `gix-packetline/src` via `etc/copy-packetline.sh`). So removing it would cause the `clippy` run in the CI `lint` job to fail, as well as likely making various other reasonable operations fail.
This removes the `gix-packetline-blocking/futures-lite` "ghost feature" added in d5dd239 (GitoxideLabs#1939), and instead: - Changes `gix-filter` to depends on `gix-packetline-blocking` without aliasing it `gix-packetline`. - Replaces uses of `gix_packetline` with `gix_packetline_blocking` in the code of `gix-filter`. Thus, this replaces the solution in GitoxideLabs#1939 for the problem discussed in GitoxideLabs#1929 (comment) with a different solution that avoids carrying a "ghost feature." In contrast, the long-standing `gix-packetline-blocking/async-io` feature added in be4de0d (GitoxideLabs#1123), though it should not be used, is *not* removed, because: - It is referenced in attributes in `gix-packetline-blocking` code (because `gix-packetline-blocking/src` is copied from `gix-packetline/src` via `etc/copy-packetline.sh`). - For that reason, removing it would cause the `clippy` run in the CI `lint` job to fail, as well as likely making various other reasonable operations fail.
Use
futures-lite
instead ofdep:futures-lite
in gix-packetline and gix-protocol.This currently affects NixOS.
Related:
dep:
syntax causes build failures in rare cases rust-secure-code/cargo-auditable#124